Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Editor] Fix AnimationTrackEditor::timeline_changed signal #95481

Merged
merged 1 commit into from
Aug 13, 2024
Merged

[Editor] Fix AnimationTrackEditor::timeline_changed signal #95481

merged 1 commit into from
Aug 13, 2024

Conversation

AThousandShips
Copy link
Member

Signal was declared with two arguments, emitted with both two and three

The signal emitted by AnimationTimelineEdit has two arguments and left as is, as is the signal from AnimationTrackEdit which is connected via a helper method in AnimationTrackEditor, unsure what the intention might have been there but that signal is never emitted by AnimationTrackEdit, but leaving that for a future cleanup and assessment

Confirmed properly that this fixes the issue, should have confirmed more properly in last attempted fix

@AThousandShips AThousandShips added this to the 4.3 milestone Aug 13, 2024
TokageItLab
TokageItLab requested changes Aug 13, 2024
View reviewed changes
Copy link
Member

@TokageItLab TokageItLab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the two signals should have different names to avoid confusion, and after then fix the incorrect bindings.

editor/animation_track_editor.cpp Show resolved Hide resolved
@AThousandShips
Copy link
Member Author

The signal for AnimationTimelineEdit and AnimationTrackEdit you mean?

@AThousandShips
Copy link
Member Author

AThousandShips commented Aug 13, 2024 •
edited
Loading

To clarify, there are tree signals:

1:

godot/editor/animation_track_editor.cpp

Line 1888 in 28e65b0

ADD_SIGNAL(MethodInfo("timeline_changed", PropertyInfo(Variant::FLOAT, "position"), PropertyInfo(Variant::BOOL, "timeline_only")));

Emitted here:

godot/editor/animation_track_editor.cpp

Line 1843 in 28e65b0

emit_signal(SNAME("timeline_changed"), ofs, mm->is_alt_pressed());

godot/editor/animation_track_editor.cpp

Line 1821 in 28e65b0

emit_signal(SNAME("timeline_changed"), ofs, mb->is_alt_pressed());

And connected to here:

godot/editor/animation_track_editor.cpp

Line 7202 in 28e65b0

timeline->connect("timeline_changed", callable_mp(this, &AnimationTrackEditor::_timeline_changed));

2:

godot/editor/animation_track_editor.cpp

Line 3360 in 28e65b0

ADD_SIGNAL(MethodInfo("timeline_changed", PropertyInfo(Variant::FLOAT, "position"), PropertyInfo(Variant::BOOL, "timeline_only")));

Never emitted

And connected to here:

godot/editor/animation_track_editor.cpp

Line 4733 in 28e65b0

track_edit->connect("timeline_changed", callable_mp(this, &AnimationTrackEditor::_timeline_changed));

3:

godot/editor/animation_track_editor.cpp

Line 7104 in 28e65b0

ADD_SIGNAL(MethodInfo("timeline_changed", PropertyInfo(Variant::FLOAT, "position"), PropertyInfo(Variant::BOOL, "timeline_only")));

Emitted here:

godot/editor/animation_track_editor.cpp

Line 6132 in 28e65b0

emit_signal(SNAME("timeline_changed"), pos, p_timeline_only);

godot/editor/animation_track_editor.cpp

Line 6105 in 28e65b0

emit_signal(SNAME("timeline_changed"), pos, false);

godot/editor/animation_track_editor.cpp

Line 3791 in 28e65b0

emit_signal(SNAME("timeline_changed"), p_pos, true, true);

godot/editor/animation_track_editor.cpp

Line 3694 in 28e65b0

emit_signal(SNAME("timeline_changed"), p_new_pos, p_timeline_only, false);

And connected to here:

godot/editor/plugins/animation_player_editor_plugin.cpp

Line 2092 in 28e65b0

track_editor->connect(SNAME("timeline_changed"), callable_mp(this, &AnimationPlayerEditor::_animation_key_editor_seek).bind(false));

@AThousandShips
[Editor] Fix AnimationTrackEditor::timeline_changed signal
4fe0d0e 
Signal was declared with two arguments, emitted with both two and three
@akien-mga akien-mga merged commit 06fbc83 into godotengine:master Aug 13, 2024
25 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips AThousandShips deleted the anim_signal_fix_2 branch August 13, 2024 13:45
@AThousandShips
Copy link
Member Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TokageItLab TokageItLab TokageItLab approved these changes

@akien-mga akien-mga akien-mga approved these changes

Assignees
No one assigned
Labels
bug regression topic:animation
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.

AnimationPlayer timeline error
3 participants
@AThousandShips @akien-mga @TokageItLab